Fix homepage localization for chip labels and translations#2275
Fix homepage localization for chip labels and translations#2275SunayKulkarni wants to merge 12 commits into
Conversation
|
Hi @SunayKulkarni — thanks for picking up #2076 and for keeping the fix focused on the homepage chip labels. I’ve requested @nettee for the pool review. Heads-up: PR #2205 is also open in the same zh-CN localization area and touches |
nettee
left a comment
There was a problem hiding this comment.
I found two issues to address before this lands: the top chip bar still leaks an English tooltip in zh-CN, and the new labelKey path currently bypasses the typed i18n keys.
| disabled={pluginsLoading || isPending || pendingPluginId !== null} | ||
| aria-selected={isActive} | ||
| title={chip.hint ?? chip.label} | ||
| title={chip.hint ?? t(chip.labelKey as any)} |
There was a problem hiding this comment.
This still leaves the HyperFrames tooltip untranslated in zh-CN. chip.hint comes from the hardcoded English string in apps/web/src/components/home-hero/chips.ts, so hovering the top-row HyperFrames tab shows Author HTML-based motion... even though homeHero.chip.hyperframesHint already exists and the lower rail uses it via homeHeroChipTitle(). Since this PR’s goal is to remove mixed-language homepage UI, please route the top tab bar through the same localized title helper (or otherwise translate the hint source) instead of reading chip.hint directly.
| { | ||
| id: 'create-plugin', | ||
| label: 'Create plugin', | ||
| labelKey: 'homeHero.chip.create-plugin', |
There was a problem hiding this comment.
The new labelKey field already contains an invalid key here: the locale dictionaries and apps/web/src/i18n/types.ts define homeHero.chip.createPlugin, not homeHero.chip.create-plugin. Right now that mismatch is hidden because labelKey is typed as plain string and the caller does t(chip.labelKey as any), so the compiler can’t catch bad keys. Please switch this entry to the existing createPlugin key and tighten labelKey to a typed i18n-key union (then drop the as any cast) so future localization regressions fail at compile time.
|
Good catch, thanks!
Let me know if anything else needs adjustment. |
nettee
left a comment
There was a problem hiding this comment.
I rechecked the follow-up head and the visible zh-CN homepage regressions are fixed. One maintainability issue remains in the new translation-key path.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| export interface HomeHeroChip { | ||
| id: string; | ||
| label: string; | ||
| labelKey: string; |
There was a problem hiding this comment.
labelKey is still widened to plain string, so this follow-up does not actually restore compile-time protection for chip translation keys. useT() only accepts keyof Dict in apps/web/src/i18n/index.tsx, but this metadata shape now allows any arbitrary string, which is how the earlier homeHero.chip.create-plugin typo slipped in unnoticed until runtime. That matters because future chip additions can reintroduce missing-translation regressions even though this PR moved the UI over to labelKey. Please type this field as a real i18n key (for example keyof Dict, or a narrower HomeHeroChipLabelKey union derived from Dict) and import that type here so bad keys fail during development instead of shipping to users.
nettee
left a comment
There was a problem hiding this comment.
I rechecked the current head and the visible zh-CN homepage regressions look fixed. One maintainability issue remains in the new chip-label metadata path.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| export interface HomeHeroChip { | ||
| id: string; | ||
| label: string; | ||
| labelKey: keyof Dict; |
There was a problem hiding this comment.
labelKey is now type-safe here, but the lower rail still does not read it: RailGroup renders homeHeroChipLabel(chip.id, t) in apps/web/src/components/HomeHero.tsx:1587, and homeHeroChipTitle() also switches on chip.id for the migrate chips at apps/web/src/components/HomeHero.tsx:1616-1619. That means the new metadata entries you changed here (create-plugin, figma, folder, template) are not actually the source of truth for the labels/tooltips users see in that rail. This matters because future homepage i18n edits can compile cleanly while silently updating only one of the two paths. Please either drive both rail labels/titles from chip metadata (for example labelKey plus a typed hint/title key) or keep the switch helpers as the single source of truth and drop the unused migrate-chip labelKey fields.
nettee
left a comment
There was a problem hiding this comment.
One blocking issue remains in the current head: the chip metadata type now requires labelKey, but the same patch removes that property from every migrate-chip entry, so this change no longer satisfies the declared HomeHeroChip contract.
| export interface HomeHeroChip { | ||
| id: string; | ||
| label: string; | ||
| labelKey: keyof Dict; |
There was a problem hiding this comment.
HomeHeroChip still requires labelKey here, but this patch removes that property from the create-plugin, figma, folder, and template entries below. That leaves HOME_HERO_CHIPS out of sync with its declared ReadonlyArray<HomeHeroChip> type, so the i18n-safety follow-up now turns into a type/build break instead of a clean refactor. Please either keep labelKey on those migrate chips, or split the model into separate create/migrate chip types so only the create chips are required to carry labelKey.
|
Restored
I see the concern about multiple sources of truth here. With For now I’ve kept this approach to avoid breaking the contract and keep the change scoped. |
nettee
left a comment
There was a problem hiding this comment.
I rechecked the current head: the chip labels/tooltips now route through the localized helpers, and the zh-CN starter copy is consistent. One follow-up regression-coverage gap remains.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| disabled={pluginsLoading || isPending || pendingPluginId !== null} | ||
| aria-selected={isActive} | ||
| title={chip.hint ?? chip.label} | ||
| title={homeHeroChipTitle(chip, t)} |
There was a problem hiding this comment.
apps/web/src/components/HomeHero.tsx:1534-1536 adds the localized label/title path, but there still is no regression test covering that zh-CN behavior. apps/web/tests/components/HomeHero.rail.test.tsx currently only checks that each chip renders, routes clicks, and toggles state; it never asserts translated text or tooltips, so the earlier labelKey typo and English HyperFrames tooltip both would have passed the suite. Because this PR's goal is to eliminate mixed-language homepage UI, please extend that rail test to render under zh-CN and assert at least one translated create-chip label plus the localized HyperFrames tooltip.
nettee
left a comment
There was a problem hiding this comment.
I rechecked the current head: the zh-CN chip labels/tooltips regression and the new locale-specific rail test both look good. One non-blocking maintainability issue remains in the chip metadata path.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| { | ||
| id: 'create-plugin', | ||
| label: 'Create plugin', | ||
| labelKey: 'homeHero.chip.createPlugin', |
There was a problem hiding this comment.
labelKey is type-safe now, but the migrate-chip metadata you changed here still is not the source of truth for what the homepage renders. RailGroup still reads homeHeroChipLabel(chip.id, t) in apps/web/src/components/HomeHero.tsx:1587, and homeHeroChipTitle() still switches on chip.id for these same create-plugin / figma / folder / template entries at apps/web/src/components/HomeHero.tsx:1616-1619. That matters because a future homepage i18n edit can update HOME_HERO_CHIPS and assume the UI changed, while the rendered label/tooltip keeps coming from the separate switch tables. Please either derive both label and tooltip from the chip metadata (for example labelKey plus a typed hintKey) or split the chip model so migrate chips do not carry localized fields that the renderer ignores.
|
I’ve taken the second approach here and split the chip model into create/migrate variants, so migrate chips no longer carry localized fields like With this change, the data model now reflects the current rendering path more accurately — create chips derive their labels from metadata, while migrate chips continue to use the existing switch-based helpers ( I’ve kept the switch helpers as the source of truth for migrate chips to keep the scope of this PR focused and avoid touching multiple rendering paths. That said, I agree that moving both label and tooltip fully to metadata (e.g., |
nettee
left a comment
There was a problem hiding this comment.
@SunayKulkarni I rechecked the current head and reviewed the changed ranges in HomeHero, the chip metadata split, the zh-CN copy update, and the new rail regression test. The localized label/title path now lines up with the current create-vs-migrate rendering split, and the added zh-CN test covers the regression this PR is fixing. Nice cleanup getting this homepage pass into a consistent state.
|
Hey @SunayKulkarni — you and @nettee did a solid job working through that chip-model split and the test coverage, and the final approval landed three days ago. 🎉 One thing standing between this and a merge: the branch now has a conflict with Let me know if anything comes up during the rebase — happy to help scope what to watch for. |
a6bfd6f to
26fa6d9
Compare
nettee
left a comment
There was a problem hiding this comment.
Two blocking regressions remain in the current head: the new create-chip label path blanks the existing live-artifact tab, and the newly added folder chip is not wired through the current action/rendering flow.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| aria-selected={isActive} | ||
| title={homeHeroChipTitle(chip, t)} | ||
| > | ||
| <span>{chip.group === 'create' ? t(chip.labelKey) : homeHeroChipLabel(chip.id, t)}</span> |
There was a problem hiding this comment.
TypeTabBar now renders create chips from chip.labelKey, but HOME_HERO_CHIPS still has an existing create-group entry (live-artifact in apps/web/src/components/home-hero/chips.ts:104-108) that only carries the old label/hint fields. Because this line takes the create branch for that chip, t(chip.labelKey) receives undefined, so the top-row live-artifact tab loses its visible label even though homeHeroChipLabel() already has the correct i18n key at apps/web/src/components/HomeHero.tsx:2586. That breaks the homepage UI this PR is trying to localize. Please either finish migrating every create chip to labelKey (for example labelKey: "homeHero.chip.liveArtifact", plus a localized hint path) or keep this renderer on homeHeroChipLabel(chip.id, t) until the create-chip metadata migration is complete.
| icon: 'folder', | ||
| group: 'migrate', | ||
| hint: 'Import an existing local folder and continue editing.', | ||
| action: { kind: 'import-folder' }, |
There was a problem hiding this comment.
This adds a new folder chip with action: { kind: import-folder }, but the rest of the changed flow still treats folder as unsupported. ChipAction in this same file does not define import-folder, HomeView only switches apply-scenario / apply-figma-migration / create-plugin / open-template-picker (apps/web/src/components/HomeView.tsx:1089-1166), and the current rail test still asserts both queryByTestId("home-hero-rail-folder") === null and findChip("folder") === undefined (apps/web/tests/components/HomeHero.rail.test.tsx:194-208). That means this PR adds a homepage chip that neither type-checks nor has a working click path. Please either drop the folder entry from this bugfix PR, or fully wire import-folder through the action union, renderer/title helpers, dispatcher, and tests before landing it.
…act to ChipScenarioPluginId
|
Fixed two chips.ts type errors (removed unused label from CreateChip interface, added example-live-artifact to ChipScenarioPluginId). All rail tests passing. |
nettee
left a comment
There was a problem hiding this comment.
I reviewed the current head's changed ranges and one merge-safe maintainability issue remains. I also couldn't rerun the web checks in this checkout because node_modules are absent.
| interface TypeTabBarProps { | ||
| activeChipId: string | null; | ||
| pendingChipId: string | null; | ||
| pendingPluginId: string | null; | ||
| pluginsLoading: boolean; | ||
| onPickChip: (chip: HomeHeroChip) => void; | ||
| } | ||
|
|
||
| function TypeTabBar({ | ||
| activeChipId, | ||
| pendingChipId, | ||
| pendingPluginId, | ||
| pluginsLoading, | ||
| onPickChip, | ||
| }: TypeTabBarProps) { | ||
| const chips = useMemo(() => chipsForGroup('create'), []); | ||
| const t = useT(); | ||
| return ( | ||
| <div className="home-hero__type-tabs" role="tablist" aria-label="Output type"> | ||
| {chips.map((chip) => { | ||
| const isActive = activeChipId === chip.id; | ||
| const isPending = pendingChipId === chip.id; | ||
| const cls = ['home-hero__type-tab']; | ||
| if (isActive) cls.push('is-active'); | ||
| if (isPending) cls.push('is-pending'); | ||
| return ( | ||
| <button | ||
| key={chip.id} | ||
| type="button" | ||
| role="tab" | ||
| className={cls.join(' ')} | ||
| data-chip-id={chip.id} | ||
| data-testid={`home-hero-rail-${chip.id}`} | ||
| onClick={() => onPickChip(chip)} | ||
| disabled={pluginsLoading || isPending || pendingPluginId !== null} | ||
| aria-selected={isActive} | ||
| title={homeHeroChipTitle(chip, t)} | ||
| > | ||
| <span>{homeHeroChipLabel(chip.id, t)}</span> | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
TypeTabBar is introduced here, but the live HomeHero tab row is still rendered through RailGroup rather than this new component. That leaves these changed lines as dead duplicate UI code, and they have already drifted from the real path (for example the hard-coded English aria-label and simpler markup here vs the localized/accessibility-aware RailGroup path below). This matters because a future homepage i18n or accessibility fix can land in TypeTabBar and never reach users. Please either remove TypeTabBar, or make it the single tab renderer and delete the overlapping tab logic from RailGroup so there is only one code path to maintain.
lefarcen
left a comment
There was a problem hiding this comment.
Hey @SunayKulkarni! The Why / What users will see writeup is clear, the Surface area checklist is filled in, and your Validation steps show you tested locally — solid context for reviewers.\n\nOne small gap: ## Bug fix verification is still just a placeholder (-). Since this carries the type/bugfix label, could you add a couple of lines on what the regression looked like before this fix and how you confirmed it is resolved? Even a short note like "homepage chips showed English text when Chinese language was set; after this fix all chips display correct zh-CN strings" alongside the existing Validation steps is enough.
nettee
left a comment
There was a problem hiding this comment.
I reviewed the current head's changed ranges and found one merge-safe maintainability follow-up in the new chip metadata path.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| label: string; | ||
| icon: IconName; | ||
| group: ChipGroup; | ||
| labelKey: keyof Dict; |
There was a problem hiding this comment.
This change makes labelKey part of the new CreateChip contract, but the homepage renderer still does not consume it: RailGroup continues to render homeHeroChipLabel(chip.id, t) in apps/web/src/components/HomeHero.tsx, and the new zh-CN test only asserts that switch-based output. That leaves two sources of truth for the same label, so a future edit can update HOME_HERO_CHIPS and assume the UI changed even though the rendered copy still comes from the separate switch table. Please either render create-chip labels directly from chip.labelKey (and extend the test to cover that path), or remove labelKey from this model until the UI actually uses it.
…le source of truth
|
Removed labelKey from CreateChip interface and all create-chip entries. The Dict import was also removed since it's no longer referenced. homeHeroChipLabel(chip.id, t) remains the single source of truth for rendered labels. The 167 TS errors from tsc --noEmit are pre-existing on main and unrelated to this PR (they're in analytics/events.ts, MemorySection.tsx, etc.). |
nettee
left a comment
There was a problem hiding this comment.
I reviewed the current head's changed ranges and the homepage localization fixes look correct, but one regression-coverage gap remains in the new zh-CN rail test.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| }); | ||
| }); | ||
|
|
||
| it('renders localized chip labels and tooltips in zh-CN', () => { |
There was a problem hiding this comment.
This regression test only asserts one always-visible create-chip label (prototype) and one tooltip (hyperframes), and it never opens the More shortcuts menu. That means the zh-CN copy for the changed migrate shortcuts (create-plugin, figma, template) can regress without this suite failing, even though the PR body says no mixed English homepage UI remains. Please extend this test to open home-hero-shortcuts-trigger under zh-CN and assert the localized labels/titles for the shortcut chips as well, so the homepage-localization guarantee is covered across both rail rows.
nettee
left a comment
There was a problem hiding this comment.
I reviewed the current head's changed ranges and the homepage localization fixes look correct overall, but one regression-coverage gap remains in the new zh-CN rail test.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| </I18nProvider> | ||
| ); | ||
|
|
||
| // Prototype chip label should be zh-CN |
There was a problem hiding this comment.
This zh-CN regression test still skips the other always-visible create chip, live-artifact. The current homepage renderer localizes that chip through homeHeroChipLabel() / homeHeroChipTitle() (homeHero.chip.liveArtifact and homeHero.chip.liveArtifactHint), and this same PR also touches its chip contract by adding example-live-artifact to ChipScenarioPluginId in apps/web/src/components/home-hero/chips.ts. Because the assertions here only cover prototype, hyperframes, and the migrate shortcuts, a future regression in the live-artifact zh-CN label or tooltip would still pass even though the PR body says no mixed-language homepage UI remains. Please add home-hero-rail-live-artifact text and title assertions in this test so the visible create-chip set is covered end to end.
nettee
left a comment
There was a problem hiding this comment.
@SunayKulkarni I rechecked the current head's changed ranges in , the zh-CN homepage copy, and the added rail regression test. The scenario wiring now matches the live chip catalog, and the zh-CN assertions cover the visible create chips plus the shortcut menu labels/tooltips this PR set out to localize. Nice job closing out the homepage mixed-language regression.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
Fixes #2076
Why
This PR addresses incomplete localization on the homepage in 0.8.0-preview.
While using the Chinese language setting, several homepage UI elements were still displayed in English. This was mainly due to hardcoded chip labels and missing or inconsistent translation entries, leading to a mixed-language experience.
Additionally, some direct English-to-Chinese translations resulted in vague or unclear meanings. This PR improves those cases by choosing more context-appropriate translations for better clarity and UX.
What users will see
When switching to Chinese:
Surface area
Screenshots
Bug fix verification
Before this fix, homepage category chips (Prototype, Image, Video, etc.) showed hardcoded English labels even when the language was set to Chinese. The HyperFrames tooltip also displayed the raw English hint string ("Author HTML-based motion...") instead of the localized version. After this fix, all chip labels and tooltips render correct zh-CN strings with no English fallbacks remaining on the homepage.
Validation
pnpm dev